🍒 [lldb] SymbolLocatorSymStore 1/2 (local lookup)#12917
Open
weliveindetail wants to merge 13 commits intoswiftlang:swift/release/6.4.xfrom
Open
🍒 [lldb] SymbolLocatorSymStore 1/2 (local lookup)#12917weliveindetail wants to merge 13 commits intoswiftlang:swift/release/6.4.xfrom
weliveindetail wants to merge 13 commits intoswiftlang:swift/release/6.4.xfrom
Conversation
…#156306) This uses split DWARF and from looking at other tests, it should not be running on Darwin or Windows. It does pass using the DIA PDB plugin but I think this is misleading because it's not actually testing the intended feature. When the native PDB plugin is used it fails because it cannot set a breakpoint. I don't see a point to running this test on Windows at all. Native PDB plugin test failures are being tracked in llvm#114906.
link.exe discards DWARF information. Other linkers on non-Windows do not. Uses the same solution as TestFrameFunctionInlined.test. This test was failing with the native PDB plugin but shouldn't have been using PDB anyway (see llvm#114906). Passes with DWARF and lld.
…lvm#153160) Relands llvm#152295. Checking for the overloads did not account for them being out of order. For example, [the failed output](llvm#152295 (comment)) contained the overloads, but out of order. The last commit here fixes that by using `-DAG`. --------- Co-authored-by: Jonas Devlieghere <jonas@devlieghere.com>
…153382) Some functions don't have the `FunctionType` set. We can't look these up and won't be able to call them, so ignore them when caching the function names. This does fix the failures caused by llvm#153160 mentioned in llvm#153160 (comment). However, in `lldb-shell::msstl_smoke.cpp` there's another failure not introduced by llvm#153160 (fixed with llvm#153386).
This PR implements `SymbolFileNativePDB::AddSymbols` which adds public symbols to the symbol table. These symbols are found in the publics stream. It contains mangled names coupled with addresses. Addresses are a pair of (segment, offset). If I understood correctly, then the segment is the section ID from the COFF header. Sections are already [constructed](https://github.com/llvm/llvm-project/blob/c48ec7fb60b5e0b4100731d75f82ea63c0ec7b45/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp#L1048) using this 1-based index ([MS docs](https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#section-table-section-headers)). This allows us to use `section_list->FindSectionByID`.
In llvm#165604, a test was skipped on Windows, because the native PDB plugin didn't set sizes on symbols. While the test isn't compiled with debug info, it's linked with `-gdwarf`, causing a PDB to be created on Windows. This PDB will only contain the public symbols (written by the linker) and section information. The symbols themselves don't have a size, however the DIA SDK sets a size for them. It seems like, for these data symbols, the size given from DIA is the distance to the next symbol (or the section end). This PR implements the naive approach for the native plugin. The main difference is in function/code symbols. There, DIA searches for a corresponding `S_GPROC32` which have a "code size" that is sometimes slightly smaller than the difference to the next symbol.
When a PDB is loaded through `target symbols add <pdb-path>`, its `m_objectfile_sp` is an `ObjectFilePDB` instead of `ObjectFilePECOFF` (the debugged module). In both the native and DIA plugin, some paths assumed that `m_objectfile_sp` is the debugged module. With this PR, they go through `m_objfile_sp->GetModule()->GetObjectFile()`. For the DIA plugin, this lead to an assertion failure (llvm#169628 (comment)) and for both plugins, it meant that the symbol table wasn't loaded.
…vm#185658) Minimal infrastructure for a the SymbolLocator plugin that fetches debug info from Microsoft SymStore repositories. This can work cross-platform and for various debug info formats in principle, but the current plan is focussed on PE/COFF on Windows with debug info in PDB files. Once we have a stable first version, we'd like to add features like download, environment variables, caching and progress feedback for users. SymbolVendorPECOFF was tailored towards DWARF debug info so far. I added code to load the PDB path from the executable (it only checked gnu_debuglink so far) and not bail out if DWARF sections are missing, so that in the PDB case we still call AddSymbolFileRepresentation() in the very end of CreateInstance(). The API test in this patch mocks the directory layout from SymStore, so it doesn't depend on SymStore.exe from the Windows SDK. It runs on all platforms that link debug info in a PDB file, which is still just Windows, but it could be cross-platform in principle. ----- Relands with minor fixes: API tests create mocked SymStore in the test's build directory. One log instruction was moved. One more object access goes through module in SymbolFile.
Deleting the executable at the end of this API test-case fails with a permission error, likely because lldb still holds a reference to the EXE. Exit explicitly to avoid that.
…lvm#187072) Deleting the inferior binary after an API test-case causes issues on one of the Windows bots. The previous the fix attempt in ca15db1 didn't succeed. We have to use isolated subfolders for each test-case. This is achieved easily by disabling SHARED_BUILD_TESTCASE.
Member
Author
|
@swift-ci please test |
Member
Author
|
Applying the patches went pretty smooth. I saw conflicts due to permanent differences downstream that I could resolve cleanly, but nothing critical. I skipped two commits that are loosely related as they'd have blown up the patch: We can skip the refactor from fb36a54 and in cdbe288 I back-ported the 1-line tablegen diff in my code. I added 2 pachtes:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Upstream LLDB gained support for fetching debug info from SymStore with a new symbol locator implementation. It would be great to bring the feature to Swift ahead of the next rebranch.
This PR contains the first half of cherry-picks:
Commits from category 2 are independent from the others and were picked first. We need them, because the SymbolLocatorSymStore implementation required a change in the SymbolFilePDB/NativePDB plugins: They are loaded through the SymbolVendorPECOFF now and not fall back to the generic SymbolVendor anymore. This was a father simple change upstream, because it had switched the default to SymbolFileNativePDB already and test inconsistencies had been fixed. This change wasn't cherry-picked, so SymbolFilePDB is the default still and required these compatibility patches.
Category 1 contains all the upstream commits that don't require HTTP lookup, because we cannot run the respective tests in Swift CI yet.